Reactive widget.value and node.inputs in vue #7095
Conversation
I would prefer that valueRef used the TValue generic, but that causes many, many awful errors. This is far preferable
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 12/02/2025, 08:52:20 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/02/2025, 08:14:20 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
| * Validates that a value is a valid WidgetValue type | ||
| */ | ||
| function validateWidgetValue(value: unknown): WidgetValue { | ||
| if (value === null || value === undefined || value === void 0) { |
There was a problem hiding this comment.
Double check, but...
| if (value === null || value === undefined || value === void 0) { | |
| if (!value) { |
or == null?
| /** | ||
| * Validates that a value is a valid WidgetValue type | ||
| */ | ||
| function validateWidgetValue(value: unknown): WidgetValue { |
There was a problem hiding this comment.
This could be a good place to do more type level validation.
| const cagRef = ref<ControlWidgetOptions>( | ||
| validateControlWidgetValue(cagWidget.value) | ||
| ) |
There was a problem hiding this comment.
What we talked about here, validate via a computed maybe?
| set(v) { | ||
| reactiveInputs.splice(0, reactiveInputs.length, ...v) | ||
| } | ||
| }) |
There was a problem hiding this comment.
This should just be in LGraphNode with a get/set.
| undefined, | ||
| inputData | ||
| ) | ||
| if (this.widgets?.[1]) widget.linkedWidgets = [this.widgets[1]] |
There was a problem hiding this comment.
This could be a one-line PR. Quick fix.
| /** Widget type (see {@link TWidgetType}) */ | ||
| type: TType | ||
| value?: TValue | ||
| valueRef?: () => Ref<boolean | number | string | object | undefined> |
There was a problem hiding this comment.
Might be able to get this to work if you use NoInfer
| input.default !== undefined | ||
| ? input.default | ||
| : input.type === 'COMBO' && |
There was a problem hiding this comment.
Optional:
| input.default !== undefined | |
| ? input.default | |
| : input.type === 'COMBO' && | |
| input.default ?? input.type === 'COMBO' && |
| icon: 'pi pi-link', | ||
| title: 'linkToGlobal', | ||
| description: 'linkToGlobalDesc' | ||
| } satisfies ControlOption |
There was a problem hiding this comment.
Double check the need for assertions here.
| if (props.controlWidget().value === mode) return | ||
| props.controlWidget().value = mode |
There was a problem hiding this comment.
Another place to try to use MaybeRefOrGetter
| <WidgetWithControl | ||
| v-else-if="widget.controlWidget" | ||
| :comp="WidgetSelectDefault" | ||
| :widget="widget as SimplifiedControlWidget<string>" |
There was a problem hiding this comment.
| const valueRef = ref(value) | ||
| if (callback) watch(valueRef, (v) => callback(v)) |
There was a problem hiding this comment.
| const valueRef = ref(value) | |
| if (callback) watch(valueRef, (v) => callback(v)) | |
| if (callback) watch(() => value, (v) => callback(v)) |
| <Button | ||
| variant="link" | ||
| size="small" | ||
| class="h-4 w-7 self-center rounded-xl bg-blue-100/30 p-0" |
There was a problem hiding this comment.
Colors should be in the theme with named tokens.
| export type SimplifiedControlWidget<T extends WidgetValue = WidgetValue> = | ||
| SimplifiedWidget<T> & Required<Pick<SimplifiedWidget<T>, 'controlWidget'>> |
There was a problem hiding this comment.
Maybe...
| export type SimplifiedControlWidget<T extends WidgetValue = WidgetValue> = | |
| SimplifiedWidget<T> & Required<Pick<SimplifiedWidget<T>, 'controlWidget'>> | |
| export interface SimplifiedWidgetWithControl<T extends WidgetValue = WidgetValue> extends SimplifiedWidget<T> { | |
| controlWidget: SimplifiedWidget<T>['controlWidget'] | |
| } |
There was a problem hiding this comment.
You'll find a good way to structure the types here.
| const valueRef = ref(value) | ||
| if (callback) watch(valueRef, (v) => callback(v)) |
A now functional WIP branch.
Built on top of the vue control_after_generate branch. ~300 lines of actual change. Likely best rebased after control_after_generate changes are merged.
┆Issue is synchronized with this Notion page by Unito